Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat/colors update #12072

Merged
merged 3 commits into from
Apr 19, 2024
Merged

Feat/colors update #12072

merged 3 commits into from
Apr 19, 2024

Conversation

vytick
Copy link
Contributor

@vytick vytick commented Apr 18, 2024

Updating colors based on export from figma

Prerequisity to #11931

@vytick vytick added the UI Issues related to stylistic/aesthetic changes label Apr 18, 2024
@vytick vytick marked this pull request as draft April 18, 2024 20:56
@vytick
Copy link
Contributor Author

vytick commented Apr 18, 2024

I've found some discrepancies between data provided and designs. Clarifying now with David which ones are correct. So I'll make it draft till resolved

Copy link
Contributor

@jvaclavik jvaclavik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested it and it looks like it doesn't broke anything in Suite. But without warranty 🫣

@@ -37,18 +37,21 @@ const light = {

// Figma Colors
backgroundAlertBlueBold: palette.lightAccentBlue600,
backgroundAlertBlueBoldAlt: palette.lightAccentBlue700,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw what does the Alt mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say ALTERNATIVE, but dunno, it was in the export.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey! The "bold-alt" tokens were added as a hotfix to the current token taxonomy due to missing colors for hover states of alert buttons. They intentionally have this generic name because we didn't want to unnecessarily add more new tokens than necessary for expanding alert buttons to new variants (red, yellow, blue). We would have to create a completely separate group with tokens "default" "hover/pressed", or rename the current tokens from "bold" to "default", which, considering the interconnectedness of tokens in various components, didn't seem like the best idea and would add unnecessary complexity. Therefore, it is primarily a temporary solution for the currently set taxonomy and naming, which is largely outdated, and that's why we are working on a new one.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This smells to me like temporarily–permanent solution 🫣

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No way.. work is already underway on the new semantics due to the shortcomings of the current one (issues with accessibility, scalability, and maintenance as our system gradually expands). Unfortunately, it's quite a broad topic that requires extensive analysis to harmonize the requirements for several touchpoints in both design and code simultaneously. Here, we need to address support for multiple platforms simultaneously, while also considering the design aspect and the need to implement a brand facelift with new colors throughout the entire system and so on..

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, got it :)

@vytick
Copy link
Contributor Author

vytick commented Apr 19, 2024

I've received new export so the colors will change.

@vytick
Copy link
Contributor Author

vytick commented Apr 19, 2024

@vytick vytick marked this pull request as ready for review April 19, 2024 10:52
@vytick vytick requested a review from jvaclavik April 19, 2024 10:52
Copy link
Contributor

@jvaclavik jvaclavik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested again and it looks fine

@vytick
Copy link
Contributor Author

vytick commented Apr 19, 2024

/rebase

Copy link

@vytick vytick merged commit 270bc57 into develop Apr 19, 2024
21 checks passed
@vytick vytick deleted the feat/colors-update branch April 19, 2024 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
UI Issues related to stylistic/aesthetic changes
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants